Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOCS] Configuration and component section overhaul #3324

Merged
merged 33 commits into from
Apr 6, 2021

Conversation

ChrisChinchilla
Copy link
Collaborator

This PR does two things…

  1. Overhauls the annotated config files to show all (and there are a lot) potential configuration points and what they mean. There are still some that are unclear, and there are todos in the comments, but it's a start. The reasoning behind this is to provide the higher level content (quoickstarts etc) and these lower level guides as we fill the more valuable gap in the middle slowly.
  2. Personally I found the previous content organisation confusing, and it focussed too much on components and not use case. So this PR also reorganises around these config files, moving the components to standardised sections under components, with repeating sections for config, API, architecture, or whatever each component needs. Then some of the more "how to" content that was under the older component sections is moved into the how to and operational guide sections, which more helpful naming. Some of the moving of that content is still in progress (i.e., how and why to apply configuration) and this just begins that, it's coming soon.

ChrisChinchilla added 12 commits March 4, 2021 14:31
Signed-off-by: ChrisChinchilla <[email protected]>
Signed-off-by: ChrisChinchilla <[email protected]>
Signed-off-by: ChrisChinchilla <[email protected]>
Signed-off-by: ChrisChinchilla <[email protected]>
Signed-off-by: ChrisChinchilla <[email protected]>
wip
Signed-off-by: ChrisChinchilla <[email protected]>
Signed-off-by: ChrisChinchilla <[email protected]>
Signed-off-by: ChrisChinchilla <[email protected]>
Signed-off-by: ChrisChinchilla <[email protected]>
Signed-off-by: ChrisChinchilla <[email protected]>
Signed-off-by: ChrisChinchilla <[email protected]>
@ChrisChinchilla
Copy link
Collaborator Author

Probably easiest to look at the preview for review…

@ChrisChinchilla
Copy link
Collaborator Author

ChrisChinchilla commented Mar 5, 2021

@nbroyles Should this also be removed?

clusters:
- namespaces:
- namespace: default
type: unaggregated

@nbroyles
Copy link
Collaborator

nbroyles commented Mar 5, 2021

@nbroyles Should this also be removed?

clusters:
- namespaces:
- namespace: default
type: unaggregated

Yup. You can also remove this:

namespaces:
# Name for the namespace
namespace: <string>
# The type of values stored by the namespace, current, valid options: [unaggregated, aggregated]
type: <string>
# How long to store metrics data
retention: <duration>
# Metrics sampling resolution
resolution: <duration>
# Configuration for downsampling options on an aggregated cluster namespace
downsample:
all: <bool>

@codecov
Copy link

codecov bot commented Mar 9, 2021

Codecov Report

Merging #3324 (ab8fedd) into master (1843061) will decrease coverage by 0.0%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #3324     +/-   ##
=========================================
- Coverage    72.4%    72.4%   -0.1%     
=========================================
  Files        1100     1100             
  Lines      102443   102443             
=========================================
- Hits        74190    74187      -3     
+ Misses      23155    23153      -2     
- Partials     5098     5103      +5     
Flag Coverage Δ
aggregator 76.9% <ø> (-0.1%) ⬇️
cluster 84.9% <ø> (ø)
collector 84.3% <ø> (ø)
dbnode 78.9% <ø> (-0.1%) ⬇️
m3em 74.4% <ø> (ø)
m3ninx 73.5% <ø> (-0.1%) ⬇️
metrics 19.8% <ø> (ø)
msg 74.6% <ø> (+0.2%) ⬆️
query 67.0% <ø> (ø)
x 80.4% <ø> (+<0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1843061...ab8fedd. Read the comment docs.

@ChrisChinchilla
Copy link
Collaborator Author

Ready for a final review @robskillington and also bringing in @wesleyk here for some input… A couple of notes after many conversations with Rob…

The m3dbnode config is pretty solid and mostly understandable. The M3 Query/Coordinator config isn't, and is still quite confusing, mostly as there seem some discrepancies between the way config works between m3dbnode setups and separated node clusters.

This is something that need resolving in the code base but also docs changes from here on in at a higher level will attempt to clarify the differences in context of implementation.

Signed-off-by: ChrisChinchilla <[email protected]>
@robskillington
Copy link
Collaborator

robskillington commented Mar 30, 2021

Overall I think this is looking really good.

I do have one major concern though having reviewed the documentation preview a few times, I think that "Architecture" is now really hard to find. You don't really expect to have "Architecture" under "Reference".

I believe actually it would be more discoverable/obvious if we did the following and split them and make them separate top level entries "Reference & Config" (to make it obvious to find config file explanations here) and "Architecture":

Reference & Config
- M3DB
 - Configuration file
- M3 Coordinator
 - API
 - Configuration file
- M3 Query
 - API
 - Configuration file

Architecture
- M3DB
 - Storage Engine
 - Sharding
 - Consistency Levels
 - Storage
 - Commit Logs And Snapshot Files
 - Peer Streaming
 - Caching
- M3 Coordinator
 - Overview
- M3 Query
 - Overview

ChrisChinchilla added 3 commits March 31, 2021 11:17
Signed-off-by: ChrisChinchilla <[email protected]>
Signed-off-by: ChrisChinchilla <[email protected]>
Copy link
Collaborator

@robskillington robskillington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ChrisChinchilla ChrisChinchilla merged commit 5a5635d into master Apr 6, 2021
@ChrisChinchilla ChrisChinchilla deleted the chrischinch/config-docs branch April 6, 2021 08:26
soundvibe added a commit that referenced this pull request Apr 6, 2021
* master:
  [DOCS] Configuration and component section overhaul (#3324)
  Pass a context to the query worker pool (#3350)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants